refactor!: back pixi's Config by rattler_config's ConfigBase<PixiConfig>#6531
Draft
wolfv wants to merge 5 commits into
Draft
refactor!: back pixi's Config by rattler_config's ConfigBase<PixiConfig>#6531wolfv wants to merge 5 commits into
wolfv wants to merge 5 commits into
Conversation
6 tasks
pixi's `Config` is no longer a parallel struct that duplicates the shared rattler configuration keys: it is now a thin `#[serde(transparent)]` wrapper around `rattler_config::config::ConfigBase<PixiConfig>` from the redesigned (unreleased) rattler_config, where `PixiConfig` is pixi's tool-specific extension. This proves out the shared-config extension mechanism end to end. What moved where: - Shared keys (default-channels, authentication-override-file, tls-no-verify, tls-root-certs, mirrors, build, channel-config, repodata-config, concurrency, proxy-config, s3-options, index-config, run-post-link-scripts, allow-symbolic/hard/ref-links, loaded_from) now live in the upstream `CommonConfig` / `ConfigBase` and are reached through `Deref` (`config.mirrors`, `config.concurrency`, ...). - Pixi-specific keys (pinning-strategy, pypi-config, detached-environments, shell, experimental, tool-platform, cache, and the deprecated change-ps1/force-activate) moved into the new `PixiConfig` extension (`config.extensions.*`), with all serde attributes (kebab-case, snake_case aliases, skip_serializing_if guards) carried over so the on-disk TOML format is unchanged. New accessors `shell()`, `experimental()`, `cache()`, `pinning_strategy()` keep call sites terse; existing accessors keep their signatures. - `tls_root_certs` is promoted to the upstream `rattler_config::config::tls::TlsRootCerts` (Webpki/System). The legacy "native"/"all" spellings deserialize as aliases of System, and pixi keeps its load-time deprecation warnings by inspecting the raw TOML document (`Config::from_toml`) and via a clap value parser for `--tls-root-certs` / `PIXI_TLS_ROOT_CERTS`. pixi_utils' feature-aware fallback logic (native-tls vs rustls) is unchanged; the LegacyNative/All match arms collapse into System. - allow-symbolic-links/allow-hard-links/allow-ref-links use the upstream CommonConfig fields (same flat key names, same format). Extensibility proof points (each covered by tests): - `Config::from_toml` uses `ConfigBase::<PixiConfig>::from_toml_str`, whose two-pass serde_ignored deserialization reports keys unknown to BOTH the common config and the extension, so the "Ignoring '...' in file" warnings keep working — including for typos of extension keys (`pinning-strateggy`) and unknown keys nested in known tables (`shell.chnge-ps1`, `pypi-config.index-urll`). New test: test_from_toml_reports_unknown_extension_keys. - `Config::set` now special-cases only the deprecated change-ps1/force-activate hard errors, detached-environments bool/path parsing (~-expansion), and the cache.* re-validation; the former ~400-line match is deleted and everything else — including extension keys like `pinning-strategy` — delegates to the upstream generic `ConfigBase::set` (TOML round-trip editor). Unknown keys produce an error listing all supported keys (shared + extension) from the `Config` trait's `keys()`. New test: test_generic_set_handles_extension_keys; also verified against the built CLI: `pixi config set pinning-strategy semver` writes `pinning-strategy = "semver"`. - `ProxyConfig::default()` no longer reads proxy env vars upstream, so pixi_config's tests are hermetic: the whole suite (including test_config_merge_multiple) now passes with and without HTTPS_PROXY/HTTP_PROXY set. Dependency patching: a `[patch.crates-io]` block redirects every rattler crate pixi consumes to the `claude/rattler-shared-config-design-m8d2gb` branch of conda/rattler (single copy of each crate, verified with `cargo tree -i rattler_conda_types`). TODO: remove once rattler_config 0.6 is released (see conda/rattler#2557) — this PR must not merge until then. Behavior changes (intentional, small): - `default_channels` is `Option<Vec<NamedChannelOrUrl>>` upstream; `Config::default_channels()` keeps returning the configured list or the consts::DEFAULT_CHANNELS fallback, and from_toml normalizes an explicitly empty list to None to preserve the old merge semantics. - `mirrors` is an `IndexMap` (was HashMap): deterministic ordering; `mirror_map()` signature updated. - `loaded_from` now lists files in load order (lowest precedence first); it previously listed them in reverse. - Editing a subkey of a non-existent s3-options bucket used to be a silent no-op; the generic editor rejects it (S3Options requires all three fields). Setting the whole bucket first works as before. - `pixi config set concurrency '<json>'` used to (buggily) write pypi-config; the generic editor handles it correctly. - PyPIConfig::is_default now also checks allow-insecure-host, so a pypi-config containing only allow-insecure-host is no longer dropped on save. - A file containing `tls-root-certs = "native"`/`"all"` still loads (with the same deprecation warning), but re-saving it writes the canonical `"system"`. Snapshot diff (config_merge_multiple): Debug output is now nested as ConfigBase { common, extensions, loaded_from }; default_channels prints as Some([...]); the (empty, serde-skipped) upstream index_config section appears; loaded_from order flipped as described above. All values are identical to the old snapshot. Tests: cargo test -p pixi_config (55 passed, with and without proxy env), cargo test -p pixi_utils (72 passed), cargo check --workspace --all-targets, cargo clippy -p pixi_config/pixi_utils/pixi_cli/ pixi_core/pixi_api/pixi_uv_context --all-targets clean, cargo fmt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EzYs4p6XJDj9QBcNB4ipR7
… schema
The pixi-side mirror of rattler_config's tests/compat.rs: that catalog
pins the contract of the shared keys; this one pins the whole pixi
schema (shared CommonConfig keys plus the PixiConfig extension) so
upgrading the shared config crate can never silently break a user's
existing config.toml.
New: crates/pixi_config/tests/compat.rs + test-data/compat/ fixtures
(kitchen-sink, snake-case-aliases, legacy-config, typos,
override-layer) + insta snapshots.
Coverage:
1. Parsing permutations — every fixture parses; (unused keys, parsed
Config) snapshotted per fixture (channel_config.root_dir normalized
so snapshots are machine-independent). snake_case aliases parse
equal to canonical with zero warnings; a realistic old pixi config
(change-ps1, force-activate, tls-root-certs = "native",
detached-environments = true, repodata-config.disable-jlap) loads
with warnings only, and the deprecated values still take effect
through the modern accessors (change_ps1()/force_activate() see the
folded shell.* values, "native" resolves to the system store);
typos of shared AND extension keys, top-level and nested, are all
reported as unused while valid neighbors survive.
2. Round-trip stability — load -> to_toml -> load is lossless and
idempotent for all five fixtures (including the legacy one: the
deprecated keys re-serialize and re-fold identically), and
serialization never invents unknown keys.
3. Editing matrix — 42 entries through the public Config::set (the
`pixi config set` code path): every shared key family plus
pinning-strategy, detached-environments (true/false/path),
tool-platform, pypi-config.*, shell.*,
experimental.use-environment-activation-cache and all cache.* keys
(absolute paths, since set re-validates). Each entry: set succeeds
on a fully populated config, the result round-trips, and set+unset
on a pristine config restores the default. Deprecated
change-ps1/force-activate stay hard errors pointing at shell.*, and
unknown keys (shared and extension typos) are rejected in both set
and unset direction.
4. Merge semantics — kitchen-sink + override layer through
Config::merge_config: scalars replace, mirrors/s3-options extend,
repodata merges field-wise (default and per-channel), pypi
index-url replaces while extra-index-urls/allow-insecure-host
extend, shell/experimental/cache merge field-wise, and the
long-standing tool-platform LOWER-layer-wins quirk is pinned with a
comment so changing it becomes a conscious decision. Merged output
snapshotted.
The catalog immediately caught a real bug: the pinned rattler commit
(33a4909) still had the repodata merge direction inverted (the lower
layer's disable-bzip2 survived an override). conda/rattler already
fixed this on the same branch in 5ded1a9 ("fix repodata merge
direction") — this commit bumps Cargo.lock to that rev, which makes
the merge test pass. None of the pre-existing pixi tests noticed the
inverted direction, which is exactly the gap this catalog closes.
Tests: cargo test -p pixi_config (55 unit + 9 compat) passes both with
proxy env scrubbed and with HTTPS_PROXY/HTTP_PROXY set; cargo check
--workspace --all-targets; clippy clean on pixi_config; cargo fmt.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EzYs4p6XJDj9QBcNB4ipR7
The parse snapshots embedded the concurrency.solves default, which is the local CPU count, so fixtures that don't set [concurrency] (e.g. snake-case-aliases.toml) produced snapshots that only passed on the machine that generated them. Normalize solves unconditionally in the snapshot helper — a conditional "only when equal to the default" would flip whenever an explicitly configured value happens to match the CPU count — and assert the explicitly configured values in code instead, where they are deterministic. Verified by running the suite pinned to a different CPU count (taskset). Same fix applied to the rattler_config catalog upstream (conda/rattler#2557). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EzYs4p6XJDj9QBcNB4ipR7
7f0f5e4 to
1b03516
Compare
Merged
3 tasks
Now that rattler-build's config PR (prefix-dev/rattler-build#2636) is merged, align pixi's rattler/rattler-build sources so the shared rattler_config / rattler_conda_types types unify: - Drop the conda/rattler [patch.crates-io] block. It pinned the pre-release rattler_config 0.5.2 / rattler 0.46 branch, which conflicts with rattler-build main (built against the released rattler_config 0.6 / rattler 0.47). rattler_config 0.6 is now released, so the patch is obsolete. - Bump rattler requirements to the released set: rattler 0.47, rattler_conda_types 0.48, rattler_config 0.6, rattler_repodata_gateway 0.30. - Patch all rattler-build crates in the tree to prefix-dev/rattler-build main (the merged config update); the crates.io rattler_build_core 0.2.8 still pins the old rattler 0.46 and is unusable until a new release is published. Lockfile re-resolved: a single copy of each shared crate from crates.io, rattler-build crates from git main, no leftover conda/rattler refs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EzYs4p6XJDj9QBcNB4ipR7
rattler_repodata_gateway 0.30 changed `Gateway::query(...).await` / `.execute().await` to return a `RepoDataQueryOutput` (repodata records plus non-fatal warnings) instead of `Vec<RepoData>`. Extract `.repodata` at the three call sites that expect a record vector: - pixi_command_dispatcher: keys/solve_conda.rs, ephemeral_env/mod.rs - pixi_cli: exec.rs Behavior-preserving: the previous API returned only records. The pixi_api search path already iterates via the output's `IntoIterator` and needs no change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EzYs4p6XJDj9QBcNB4ipR7
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to #6528 (which merged the leaf-type migration from #6144). This PR contains the final form of the shared-config design plus a compatibility test catalog:
Commit 1 — the final form.
pixi_config::Configis no longer a parallel struct — it is a#[serde(transparent)]wrapper aroundrattler_config::ConfigBase<PixiConfig>(net −630 lines):PixiConfigis the tool extension the design was made for:pinning-strategy,pypi-config,detached-environments,shell,experimental,tool-platform,cache, and the deprecatedchange-ps1/force-activate— all with their original serde attributes, so the on-disk format is unchanged. Shared keys come from upstreamCommonConfig(reached viaDeref, soconfig.mirrorsetc. keep working).tls_root_certsand theallow-*-linkstrio are promoted upstream (the follow-ups flagged in refactor: switch to rattler_config #6144). Legacy"native"/"all"spellings keep parsing and keep their deprecation warnings.pixi config listwarnsIgnoring 'pinning-strateggy, shell.chnge-ps1' in …);pixi config set pinning-strategy semver(an extension key) works through the upstream generic editor with zero per-key code (the old ~400-linesetmatch is deleted, keeping only 3 pixi-specific special cases);test_config_merge_multiplepasses withHTTPS_PROXYset (upstreamProxyConfig::default()env-leak fix).Commit 2 — compatibility test catalog (mirrors conda/rattler#2557): full-schema fixtures (kitchen sink, snake_case aliases, a legacy pre-0.40 config, typos), round-trip losslessness, a 42-entry set/unset edit matrix, and snapshotted merge semantics — so future changes can't silently break existing user configs. Snapshots normalize machine-dependent values (
concurrency.solves= CPU count,channel_config.root_dir).The redesigned
rattler_configthis builds on has now shipped as 0.6.0 on crates.io (conda/rattler#2557, merged and released).Although
rattler_config0.6 is on crates.io, this PR cannot yet switch off the git[patch.crates-io]and onto the published versions, because of a release-ordering chain:pixi's pixi-build backends depend on
rattler_build_coreas a library. The latest publishedrattler_build_core(0.2.8) still pinsrattler 0.46/rattler_config ^0.5.2/rattler_conda_types ^0.47, which cannot coexist with pixi's directrattler 0.47/rattler_config 0.6/rattler_conda_types 0.48:So this PR keeps the
[patch.crates-io]pointing the rattler crates at conda/rattler's branch (which carries the identical 0.6 code at pre-release version numbers, satisfyingrattler_build_core's ranges) until prefix-dev/rattler-build#2636 merges and publishes arattler_build_corebuilt against rattler 0.47 / rattler_config 0.6. Once that lands, the final flip here is a pure dependency swap: delete the patch block, bump the workspace version requirements +rattler_build_core,cargo update. The branch has been rebased onto currentmainand recompiles clean under the patch.How Has This Been Tested?
cargo check -p pixi_configclean after rebasing onto currentmain(resolving rattler crates from the conda/rattler branch).cargo treeconfirms a single copy of every rattler crate under the patch.AI Disclosure
Tools: Claude Code
Checklist:
schema/model.py(no schema impact — serialized config format unchanged)